-
Notifications
You must be signed in to change notification settings - Fork 3.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(opensearch): always create CloudWatch Logs resource policy when logging is enabled #28707
fix(opensearch): always create CloudWatch Logs resource policy when logging is enabled #28707
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this! This looks good to me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically good to go @sakurai-ryo. Asking for a bit more clarification in the readme.
When logging is enabled for the domain, the CloudWatch Logs resource policy is created by default. | ||
This resource policy is necessary for logging, but since only a maximum of 10 resource policies can be created per region, | ||
the maximum number of resource policies may be a problem when enabling logging for several domains. | ||
By setting the `suppressLogsResourcePolicy` option to true, you can suppress the creation of a CloudWatch Logs resource policy. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you also hint at the side-effects of adding this prop? What does it mean to not have a resource policy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right.
I added a description about this to readme and jsdoc.
…opensearch-suppress-resource-policy
Pull request has been modified.
@kaizencc |
…opensearch-suppress-resource-policy
…com/sakurai-ryo/aws-cdk into opensearch-suppress-resource-policy
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes LGTM. Thanks for the contribution and clear documentation!
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
…ogging is enabled (aws#28707) This PR adds an option to suppress the creation of logs resource policy when logging is enabled. ### Description Currently, a CloudWatch Logs resource policy is created by default when the Domain logging is enabled. However, since only ten resource policies can be created per region, deploying multiple Domains may cause errors. The `tryRemoveChild` method can be used as a workaround to delete custom resources, but a better user experience is desirable. ```ts const domain = new opensearch.Domain(this, 'Domain', domainProps); const domainResource = domain.node.defaultChild as opensearch.CfnDomain; domainResource.addOverride('DependsOn', undefined); // remove dependency on the custom resource domain.node.children .filter(child => child instanceof AwsCustomResource) .forEach(value => domain.node.tryRemoveChild(value.node.id)); ``` So, I add an option to suppress the creation of resource policies. This option allows users to reuse a broader resource policy and successfully deploy several domains. https://docs.aws.amazon.com/opensearch-service/latest/developerguide/createdomain-configure-slow-logs.html#:~:text=Resource%22%3A%20%22cw_log_group_arn%3A*%22%7D%5D%7D%27-,Important,-CloudWatch%20Logs%20supports Closes aws#23637 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
…ogging is enabled (#28707) This PR adds an option to suppress the creation of logs resource policy when logging is enabled. ### Description Currently, a CloudWatch Logs resource policy is created by default when the Domain logging is enabled. However, since only ten resource policies can be created per region, deploying multiple Domains may cause errors. The `tryRemoveChild` method can be used as a workaround to delete custom resources, but a better user experience is desirable. ```ts const domain = new opensearch.Domain(this, 'Domain', domainProps); const domainResource = domain.node.defaultChild as opensearch.CfnDomain; domainResource.addOverride('DependsOn', undefined); // remove dependency on the custom resource domain.node.children .filter(child => child instanceof AwsCustomResource) .forEach(value => domain.node.tryRemoveChild(value.node.id)); ``` So, I add an option to suppress the creation of resource policies. This option allows users to reuse a broader resource policy and successfully deploy several domains. https://docs.aws.amazon.com/opensearch-service/latest/developerguide/createdomain-configure-slow-logs.html#:~:text=Resource%22%3A%20%22cw_log_group_arn%3A*%22%7D%5D%7D%27-,Important,-CloudWatch%20Logs%20supports Closes #23637 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
This PR adds an option to suppress the creation of logs resource policy when logging is enabled.
Description
Currently, a CloudWatch Logs resource policy is created by default when the Domain logging is enabled.
However, since only ten resource policies can be created per region, deploying multiple Domains may cause errors.
The
tryRemoveChild
method can be used as a workaround to delete custom resources, but a better user experience is desirable.So, I add an option to suppress the creation of resource policies.
This option allows users to reuse a broader resource policy and successfully deploy several domains.
https://docs.aws.amazon.com/opensearch-service/latest/developerguide/createdomain-configure-slow-logs.html#:~:text=Resource%22%3A%20%22cw_log_group_arn%3A*%22%7D%5D%7D%27-,Important,-CloudWatch%20Logs%20supports
Closes #23637
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license